-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: write snapshots to disk even if FS is mocked #7080
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! It would be good to solve this more generally and across all packages
Flow is being dumb and not understanding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 but I agree with @mattphillips
Of course, we've had that issue for years, though. And I noted it in the OP. Ideas welcome! :D |
@SimenB can we re-implement this with |
Ah, good idea! |
fddce09
to
b3328a9
Compare
@mattphillips @thymikee updated |
@@ -57,6 +66,47 @@ module.exports = ({template}) => { | |||
|
|||
path.parentPath.replaceWithSourceString('jestNow'); | |||
} | |||
if ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lots of copypasta here, should probably refactor this at some point
b3328a9
to
1265cd0
Compare
Codecov Report
@@ Coverage Diff @@
## master #7080 +/- ##
=======================================
Coverage 67.91% 67.91%
=======================================
Files 248 248
Lines 9507 9507
Branches 6 5 -1
=======================================
Hits 6457 6457
Misses 3048 3048
Partials 2 2
Continue to review full report at Codecov.
|
@SimenB mind fixing prettier and merge this? :) |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
We have a general problem in that when a user messes with globals, or requires node core modules, they get the same copy as we get. That means we have to make sure to work, even if users do
global.Promise = undefined
orfs.writeFileSync = () => {}
.We really should solve it generally (stick all globals and core modules on some
Symbol
known only to Jest?), but this solves it for the case of snapshots and FS mocking.Fixes #5270. Without the changes in this PR, the snapshot would be reported as written, but not actually written to disk.
Test plan
Added integration test